Remove ihp-log package, use fast-logger directly#2600
Remove ihp-log package, use fast-logger directly#2600mpscholten wants to merge 4 commits intomasterfrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eadbc197be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Log.error ("SSC: Failed parsing action for " <> componentName <> ": " <> errorText) | ||
| Log.debug ("SSC: Invalid payload: " <> tshow actionPayload) | ||
| log ("SSC: Failed parsing action for " <> componentName <> ": " <> errorText) | ||
| log ("SSC: Invalid payload: " <> tshow actionPayload) |
There was a problem hiding this comment.
The new log helper writes every message unconditionally, so this line now emits the full client-supplied actionPayload whenever JSON decoding fails; previously this path used Log.debug and was filtered out in production by default. In production this can leak sensitive user input into centralized logs and lets malformed/probing traffic generate large log volumes, so this payload dump should be debug-gated or removed.
Useful? React with 👍 / 👎.
| poller <- pollForJob enablePollerTriggerRepair pool tableName pollInterval onNewJob | ||
| subscription <- liftIO $ pgListener |> PGListener.subscribe (channelName tableNameBS) (const (do | ||
| Log.debug ("Received pg_notify for " <> tableName) | ||
| ?context.logger (toLogStr ("Received pg_notify for " <> tableName) <> "\n") |
There was a problem hiding this comment.
This callback now logs every Postgres notification unconditionally, while the previous code used Log.debug for this message. On busy queues, each enqueued/updated job will produce an extra log write, which can create substantial log noise and I/O overhead in production; this should remain debug-only (or be removed) to avoid degrading throughput and observability signal quality.
Useful? React with 👍 / 👎.
Replace the ihp-log wrapper package with direct usage of FastLogger from the fast-logger package. The ihp-log abstraction (log levels, formatters, destinations) was unused in practice — everything logged to stdout with the default formatter, and log levels only mattered for query timing optimization. Changes: - FrameworkConfig.logger and ModelContext.logger are now FastLogger (LogStr -> IO ()) directly from System.Log.FastLogger - Add ModelContext.debugMode :: Bool to replace log-level-based query timing skip (set from DEBUG env var) - buildFrameworkConfig takes a FastLogger parameter; withFrameworkConfig uses withFastLogger bracket for lifecycle management - Request logger middleware wired directly via Callback (perfect type match with FastLogger) - All Log.debug/info/warn/error calls replaced with direct ?context.logger (toLogStr msg <> "\n") - Delete ihp-log/ package entirely (-812 net lines) - Update Guide docs for new logging API Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Append "\n" in buildFrameworkConfig and DevServer logger creation instead of at every call site - Rename ModelContext.debugMode to .queryLoggingEnabled - Add noopLogger to IHP.ModelSupport.Types, replace ~15 inline (\_ -> pure ()) definitions across test files Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
mpscholten
force-pushed
the
remove-ihp-log
branch
from
3878ad2 to
be584b8
Compare
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Core Size & Compile Allocations Benchmark
HTTP Latency (GET /, 5000 reqs, 10 concurrent)
Top 10 modules (this PR)
|
1 participant
Summary
ihp-logwrapper package entirely (-812 net lines)FastLogger(LogStr -> IO ()) fromfast-loggerdirectly — no wrapper types, no log levels, no custom formattersFrameworkConfig.loggerandModelContext.loggerare nowFastLoggerdirectlyModelContext.debugMode :: Bool(fromDEBUGenv var) instead of checking log levelwithFastLoggerbracket inwithFrameworkConfigRequestLogger.Callback logger(perfect type match)buildFrameworkConfignow takes aFastLoggerparameterMotivation
ihp-logwrappedfast-loggerwith log levels, formatters, and destinations. In practice none of this was used:Log.fatalandLog.unknownwere never calledBreaking changes for IHP app developers
Log.debug/info/warn/error "msg"→?context.logger (toLogStr "msg" <> "\n")import qualified IHP.Log as Log→import System.Log.FastLogger (toLogStr)LogLevel,LoggerSettings,newLoggerno longer existbuildFrameworkConfignow requires aFastLoggeras first argumentTest plan
IHP.Logreferences in source code🤖 Generated with Claude Code